fix(rag): require co-located medication evidence#581
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughExpanded medication dose intent detection and evidence matching for amount, route, and frequency queries. Updated RAG coverage gating to require co-located medication evidence, with tests covering new units, schedules, route failures, and split evidence. ChangesMedication dose evidence
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Query
participant ClinicalSearch
participant RAGCoverageGate
participant RetrievalEvidence
Query->>ClinicalSearch: classify amount, route, and frequency intent
ClinicalSearch-->>RAGCoverageGate: structured medication dose intent
RAGCoverageGate->>RetrievalEvidence: check co-located dose and context evidence
RetrievalEvidence-->>RAGCoverageGate: matching evidence attributes
RAGCoverageGate-->>Query: accept fast path or return rejection reason
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/clinical-search.ts (1)
750-761: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMedication dose unit/frequency vocabularies are duplicated across
clinical-search.tsandrag.ts. The amount-unit spelling list (mg/mcg/micrograms/milligrams/ug/µg) and the frequency-token list (once/twice/daily/…/qid/every-N-hours) are each maintained independently in two places; this very PR had to edit both in lockstep to add the same new spellings, which is the classic sign of future drift.
src/lib/clinical-search.ts#L750-L761: canonical site — export the unit pattern (and ideally the frequency pattern near Line 1190) as shared constants.src/lib/clinical-search.ts#L1186-L1191: reuse the same exported unit/frequency constants instead of re-listing the spellings.src/lib/rag.ts#L2764-L2766: import the shared unit constant fromclinical-search.tsinstead of re-declaring the identical regex.src/lib/rag.ts#L2775-L2781: import the shared frequency constant fromclinical-search.tsinstead of re-declaring an overlapping regex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/clinical-search.ts` around lines 750 - 761, Deduplicate medication vocabulary regexes by exporting shared unit and frequency pattern constants from clinical-search.ts, anchored at hasDoseEvidenceSupport and the frequency logic around lines 1186-1191. Update clinical-search.ts to reuse those constants, and in src/lib/rag.ts lines 2764-2766 and 2775-2781 import and reuse the shared unit and frequency constants instead of redeclaring the patterns; preserve the existing matching behavior.src/lib/rag.ts (1)
2936-2991: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid, test-verified co-location logic — consider extracting the repeated contextual-check pattern.
Manually traced this cascade against every provided medication_dose_risk test case (split-evidence rejection, route-only rejection, entity-crossing rejection, dose+route acceptance, microgram-only acceptance) and every reason matches. The five
top.some((result) => hasDoseEvidenceSupport(result) && <check> && medicationDoseQueryContext(query, result).matched)blocks differ only by the injected check function, which is a good candidate for a small helper to reduce duplication and ease future attribute additions (e.g., a future "duration" attribute).♻️ Extract a contextual-evidence helper
+const hasContextualEvidence = ( + top: SearchResult[], + query: string, + check: (result: SearchResult) => boolean = () => true, +) => + top.some( + (result) => + hasDoseEvidenceSupport(result) && check(result) && medicationDoseQueryContext(query, result).matched, + ); + - const hasContextualDoseEvidence = top.some( - (result) => hasDoseEvidenceSupport(result) && medicationDoseQueryContext(query, result).matched, - ); - const hasContextualDoseAmount = top.some( - (result) => - hasDoseEvidenceSupport(result) && - hasDoseAmountEvidenceForGate(result) && - medicationDoseQueryContext(query, result).matched, - ); + const hasContextualDoseEvidence = hasContextualEvidence(top, query); + const hasContextualDoseAmount = hasContextualEvidence(top, query, hasDoseAmountEvidenceForGate);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rag.ts` around lines 2936 - 2991, Extract the repeated contextual `top.some` checks in the `medication_dose_risk` branch into a small helper that accepts the result collection and attribute-specific evidence predicate, while consistently requiring `hasDoseEvidenceSupport` and a matching `medicationDoseQueryContext`. Replace `hasContextualDoseEvidence`, `hasContextualDoseAmount`, `hasContextualRoute`, `hasContextualFrequency`, and the corresponding co-location checks in this branch with the helper, preserving all existing acceptance and reason behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/clinical-search.ts`:
- Around line 750-761: Deduplicate medication vocabulary regexes by exporting
shared unit and frequency pattern constants from clinical-search.ts, anchored at
hasDoseEvidenceSupport and the frequency logic around lines 1186-1191. Update
clinical-search.ts to reuse those constants, and in src/lib/rag.ts lines
2764-2766 and 2775-2781 import and reuse the shared unit and frequency constants
instead of redeclaring the patterns; preserve the existing matching behavior.
In `@src/lib/rag.ts`:
- Around line 2936-2991: Extract the repeated contextual `top.some` checks in
the `medication_dose_risk` branch into a small helper that accepts the result
collection and attribute-specific evidence predicate, while consistently
requiring `hasDoseEvidenceSupport` and a matching `medicationDoseQueryContext`.
Replace `hasContextualDoseEvidence`, `hasContextualDoseAmount`,
`hasContextualRoute`, `hasContextualFrequency`, and the corresponding
co-location checks in this branch with the helper, preserving all existing
acceptance and reason behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 08a50c81-6a84-4b5e-ac77-f68d33098f28
📒 Files selected for processing (4)
docs/branch-review-ledger.mdsrc/lib/clinical-search.tssrc/lib/rag.tstests/retrieval-query-variants.test.ts
Summary
how much,how often, microgram units, and route wordingVerification
npm run verify:pr-local -- --dry-run --files src/lib/clinical-search.ts,src/lib/rag.ts,tests/retrieval-query-variants.test.ts,docs/branch-review-ledger.md: selected runtime, format, lint, typecheck, full tests, build, and offline RAG. The executable combined gate was not repeated because this junction-based checkout needs over 10 minutes for the full unit suite alone; its constituents are listed below.npm run verify:ui: not applicable because the duplicate UI test change was removed; merged PR feat(ui): single compact site-wide privacy notice below the composer #576 separately passed critical and advisory UI.npm run verify:release: not run because it includes provider-backed release checks; the authorized final provider gates are being run separately.npm run eval:retrieval:quality: running in Eval Canary run 29251155102 on this exact branch head.npm run eval:rag -- --limit 15andnpm run eval:quality -- --rag-only: not run separately because no synthesis prompt or answer prose changed; the hosted answer-quality subset is running in Eval Canary run 29251155102.npm run check:production-readiness: local fail-closed run completed with provider variables deliberately cleared; final configured production run remains pending after deployment.npx --no-install vitest run tests/clinical-search.test.ts tests/deep-memory.test.ts tests/retrieval-query-variants.test.ts tests/retrieval-selection.test.ts --reporter=dot: 4 files passed, 143/143 tests passed.npm run eval:rag:offline: 21 files passed, 265/265 tests passed, and 36 fixture schemas passed.npm run typecheck: passed..\node_modules\.bin\eslint.cmd src/lib/clinical-search.ts src/lib/rag.ts tests/retrieval-query-variants.test.ts --max-warnings=0: passed with zero warnings.npm test: 211 files passed, 1 skipped; 1,946 tests passed, 1 skipped..\node_modules\.bin\prettier.cmd --check src/lib/clinical-search.ts src/lib/rag.ts tests/retrieval-query-variants.test.ts docs/branch-review-ledger.md: all matched files passed.git diff --check origin/main...HEAD: passed with no whitespace errors.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
npm run verify:cheappassed runtime/config/format-adjacent guards, lint, and typecheck, then its 10-minute host wrapper expired while the full unit suite was still running. The same full suite passed independently under a longer bound.